-
Notifications
You must be signed in to change notification settings - Fork 429
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Make sure column names are always titlized #368
Conversation
Thanks for looking into this, however, before this can be merged, tests have to pass. Can you have a careful look at them? |
I've had a quick look. I have to admit that the intent of some of the tests is not clear to me. And there are some that I don't agree with on first glance. I will have a more careful look. In the meantime, can you take a look at what I've done? As I said it is probably a very naive fix (not meant for merging just yet) and maybe you can also provide some pointers. |
@@ -434,7 +434,7 @@ def verbose_name(self): | |||
|
|||
# This is our reasonable fallback, should the next section not result | |||
# in anything useful. | |||
name = title(self.name.replace('_', ' ')) | |||
name = self.name.replace('_', ' ') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this change is applied, the second part of the first sentence of the docstring of this method is not true anymore. Please look carefully at the docstrings too when modifying these kinds of things.
This might reflect assumptions made before things were clarified like in #249, and should be changed accordingly. However, I think we should not always titlize. If I provide an explicit |
@palm86 did you see my comments? |
#382 is a better approach, closing this one in favour of that PR. |
Here is a naive fix for #249 . I'd be happy to dig in some more. It works for me.